Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Replace deprecated gulp-util #5

Merged
merged 17 commits into from
Mar 7, 2018
Merged

Replace deprecated gulp-util #5

merged 17 commits into from
Mar 7, 2018

Conversation

ScorpioCoding
Copy link
Contributor

Hi there,

Created a hotfix for the gulp-util deprecation issue
Installed dependecies

  • plugin-error as replacement for gutil.PluginError
  • fancy-log to replace gutil.log
  • ansi-colors to replace gutil.colors
  • vinyl to replace gutil.File

files modified are:
.gitignore
--- added package-lock.json that occures when using VS code and SourceTree Git manager
package.json
--- added dependencies
index.js
--- mod code

I hope that my first try is a succes?
my firdst pull-request....lol

kind regards
scorpiocoding - kribo

@denOldTimer
sc: mod: gitignore file - package-lock.json
9d9c487
@denOldTimer
sc mod: package.json - add: dep. plugin-error
8a01e7e
@denOldTimer
sc mod: index.js - plugin-error repalcement for gutil.pluginError
1e96d21
@denOldTimer
sc mod: package.json - add: dep. fancy-log
4b01228
@denOldTimer
sc mod: index.js - fancy-log repalcement for gutil.log
b5d54a3
@denOldTimer
sc mod: package.json - add: dep. ansi-colors
cbe16ae
@denOldTimer
sc mod: index.js - ansi-colors repalcement for gutil.colors
5b8a2df
@denOldTimer
sc mod: package.json - add: dep. vinyl
979e1bf
@denOldTimer
sc mod: index.js - vinyl repalcement for gutil.File
999c510
@denOldTimer
sc mod: index.js - naming of log to fancyLog
7e03338
@denOldTimer
sc mod: index.spec.js - fancy-log to replace gutil.log
aa32eb1
@denOldTimer
sc mod: index.spec.js - vinyl to replace gutil.File
202a411
@denOldTimer
sc mod: index.js - renaming var vinyl to File
6c49c54
@denOldTimer
sc mod: index.spec.js - renaming var vinyl to File
d3cd080
@meeroslav
Copy link
Owner

@ScorpioCoding, thank's for help. Please fix the failing unit tests in order to accept this pull request.

@demurgos
Copy link

demurgos commented Jan 3, 2018 •
edited
Loading

@meeroslav
Hi there, I'm tracking the gulp-util migration in this issue, check it for more information. I can't send PRs to everyone but I subscribed to this PR and will review it once CI passes if you want.

@ScorpioCoding
I posted some comments regarding your PRs in your run-sequence PR. This PR here is not a duplicate so if you fix the issues it should be fine (if @meeroslav agrees to merge).

@ScorpioCoding
Copy link
Contributor Author

@meeroslav
would luv to fix the unit testing but it keeps giving me a nodejs error - set env variable.
Zo where do I set the env variable?
Or what simple thing have I forgotten?
Can u advice plz?

@ScorpioCoding
Copy link
Contributor Author

@demurgos
I read your comments and I have taken them to hart and modded a few open PR's
Thnx for the sugestions..

@meeroslav
Copy link
Owner

meeroslav commented Jan 4, 2018 •
edited
Loading

@denOldTimer or @ScorioCoding, you can check travis for more details https://travis-ci.org/meeroslav/gulp-inject-partials/jobs/324991485.

You cannot simply replace gutil.log with fancyLog in the beforeEach as you then simply create new pointer which is not linked to anything. Since fancyLog calls console.log under the hood, you can override console.log instead of old gutil.log.

@meeroslav meeroslav changed the title Scorpiocoding Replace deprecated gulp-util Jan 4, 2018
@ScorpioCoding
Copy link
Contributor Author

@meeroslav
Keep getting an AssertionError on line 127 of index.spec.js

demurgos
demurgos suggested changes Jan 4, 2018
View reviewed changes
src/index.spec.js Outdated
@@ -12,15 +18,15 @@ describe('gulp-inject-partials', function(){
var logOutput = [];

beforeEach(function () {
log = gutil.log;
log = fancyLog.log;
Copy link

@demurgos demurgos Jan 4, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd recommend to use fancyLog.info (everywhere in this package) because of this kind of test relying on Node's cache to patch shared dependencies. (fancyLog.log does not exist)

I personally consider it a bit brittle so I exposed the logger on the main module in another PR. Finally, a solution would be to use proxy-require to stub fancy-log.

@meeroslav
Copy link
Owner

I will fix this on the master, since it's simpler that way. Sorry for waiting so long.

@meeroslav meeroslav merged commit 0b86150 into meeroslav:master Mar 7, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@demurgos demurgos demurgos requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@ScorpioCoding @meeroslav @demurgos @denOldTimer